-
Notifications
You must be signed in to change notification settings - Fork 187
Plugin Support for Java SDK #2761
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
| * @see PluginBase | ||
| */ | ||
| @Experimental | ||
| public interface ClientPlugin extends ClientPluginCallback { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this WorkflowClientPlugin or is it meant to work for ScheduleClient too (and the upcoming standalone activity client and Nexus operation client)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's keep it scoped to just workflows. I think it should be clear now, after I moved it to the appropriate package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This means that schedule clients can't have plugins? Clients doing schedule things can have plugins in every other SDK (same concern for standalone activity clients coming and Nexus operation clients coming)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah i see, i was missing that in Python the big huge Client also includes scheduling stuff, whereass in java they are separate. I'll take a closer look at that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 And note, there are more clients expected to come with standalone activity client and Nexus operation client, but they all will use the same service stubs
temporal-sdk/src/main/java/io/temporal/client/ClientPlugin.java
Outdated
Show resolved
Hide resolved
temporal-sdk/src/main/java/io/temporal/client/ClientPlugin.java
Outdated
Show resolved
Hide resolved
| * Callback interface for client plugins to participate in service stubs creation. This interface | ||
| * is implemented by {@code ClientPlugin} in the temporal-sdk module. | ||
| */ | ||
| interface ClientPluginCallback { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should just consider making a ServiceStubPlugin separate from a WorkflowClientPlugin and if someone wants to implement both, they can. We can discuss whether a plugin here automatically propagates to clients if it implements the proper interface (I think it should, same way client plugins should automatically propagate to workers if the they implement the right interfaces).
| * @return the workflow service stubs | ||
| */ | ||
| static WorkflowServiceStubs newServiceStubs( | ||
| @Nonnull WorkflowServiceStubsOptions options, @Nonnull List<?> plugins) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should accept the plugins on the options and therefore only mutate existing stub construction methods instead of adding a new one. All other SDK implementations of plugins do (acknowledging that plugins should not mutate the plugin list or it's undefined behavior). This makes it easy for users building options separate from where they call this. It also makes it easy to properly apply to the existing overloads like newConnectedServiceStubs. This was done for client, I think it's worth doing here.
temporal-serviceclient/src/main/java/io/temporal/serviceclient/WorkflowServiceStubs.java
Outdated
Show resolved
Hide resolved
temporal-sdk/src/main/java/io/temporal/common/SimplePluginBuilder.java
Outdated
Show resolved
Hide resolved
temporal-sdk/src/main/java/io/temporal/common/plugin/WorkerPlugin.java
Outdated
Show resolved
Hide resolved
temporal-sdk/src/main/java/io/temporal/common/plugin/WorkerPlugin.java
Outdated
Show resolved
Hide resolved
temporal-sdk/src/main/java/io/temporal/common/plugin/WorkerPlugin.java
Outdated
Show resolved
Hide resolved
temporal-sdk/src/main/java/io/temporal/client/ClientPlugin.java
Outdated
Show resolved
Hide resolved
temporal-sdk/src/main/java/io/temporal/common/plugin/WorkerPlugin.java
Outdated
Show resolved
Hide resolved
…tdownWorkerFactory to SimplePlugin.
| * @see WorkerPlugin | ||
| */ | ||
| @Experimental | ||
| public class SimplePlugin implements ClientPlugin, WorkerPlugin { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pedantic, but if we're not expecting this to be user-instantiable directly and only via builder or extension, could mark it as abstract (not worried about us adding future client/worker plugin methods we forget to implement here, because to be backwards compatible, all future client/worker plugin methods we add will have to have a default)
| } | ||
|
|
||
| /** Builder for creating {@link SimplePlugin} instances with declarative configuration. */ | ||
| public static final class Builder { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For a "simple plugin", in addition to interceptors and context propagators, I think users should also be able to directly/simply set data converters, activities, workflows, and Nexus services
Plugin System for Java Temporal SDK
What was changed
Added a plugin system for the Java Temporal SDK, modeled after the Python SDK's plugin architecture but adapted to Java idioms.
New Files (
temporal-sdk/src/main/java/io/temporal/common/plugin/)ClientPlugin.java- Client-side plugin interfaceWorkerPlugin.java- Worker-side plugin interfacePluginBase.java- Convenience base class implementing bothSimplePluginBuilder.java- Builder for declarative plugin creationModified Files
WorkflowServiceStubs.java- AddednewServiceStubs(options, plugins)andClientPluginCallbackinterfaceWorkflowClientOptions.java- Addedpluginsfield with builder methodsWorkflowClientInternalImpl.java- AppliesClientPlugin.configureClient()during creationWorkerFactory.java- Full plugin lifecycle (configuration, execution, shutdown)Test Files (
temporal-sdk/src/test/java/io/temporal/common/plugin/)PluginTest.java- Core plugin interface testsSimplePluginBuilderTest.java- Builder API testsWorkflowClientOptionsPluginTest.java- Options integration testsWhy?
The plugin system provides a higher-level abstraction over the existing interceptor infrastructure, enabling users to:
Checklist
Closes Plugin support #2626, and tracked in Plugins to support controlling multiple configuration points at once features#652.
How was this tested:
PluginTest.java,SimplePluginBuilderTest.java,WorkflowClientOptionsPluginTest.java)Any docs updates needed?
Design Decisions
1. No Base
PluginInterfaceDecision:
ClientPluginandWorkerPlugineach define their owngetName()method independently, rather than sharing a basePlugininterface.Rationale: This matches the Python SDK's design. Python has separate
ClientPluginandWorkerPluginwithname()on each. I initially had a basePlugininterface but removed it to simplify.Alternative considered: I could add a shared
Plugininterface with justgetName(). This would allowList<Plugin>instead ofList<?>for storage. However, this adds an interface that serves no purpose other than type convenience, and Python doesn't have it.2.
ClientPluginCallbackInterface (Module Boundary)Decision: A
ClientPluginCallbackinterface exists intemporal-serviceclient, whichClientPlugin(intemporal-sdk) extends.Rationale: This is required due to Java's module architecture:
temporal-serviceclientcontainsWorkflowServiceStubstemporal-sdkdepends ontemporal-serviceclient(not vice versa)WorkflowServiceStubs.newServiceStubs(options, plugins)needs to call plugin methodsThis is the one structural difference from Python, which uses a single-package architecture where everything can import everything else.
3.
PluginBaseConvenience ClassDecision: I provide an abstract
PluginBaseclass that implements bothClientPluginandWorkerPlugin.Rationale: This is a common Java pattern (like
AbstractListforList).4.
SimplePluginBuilderwith PrivateSimplePluginDecision: I provide a builder for creating plugins declaratively, with the implementation class kept private.
Rationale:
SimplePluginis an implementation detail - users interact with the builder5. No ServiceLoader Discovery
Decision: I do not include ServiceLoader-based plugin discovery.
Rationale:
plugins=[]SimplePluginBuilderWe could consider adding it in though if there is interest.
Example Usage
Custom Plugin
Using SimplePluginBuilder
Client/Worker with Plugins
Open Questions
@Experimental? - I've marked all public APIs as@Experimentalto allow iteration. Is this appropriate?